LedFeedbackClass fix led pin not initialized #22
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22 +/- ##
=======================================
Coverage 90.27% 90.27%
=======================================
Files 7 7
Lines 720 720
Branches 86 86
=======================================
Hits 650 650
Misses 70 70 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Memory usage change @ 8fef0c7
Click for full report table
Click for full report CSV |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where an uninitialized LED pin was causing an OPTA relay to malfunction by continuously turning on and off. The fix ensures proper pin initialization and validation before LED operations.
- Initialize LED pin with an invalid value and validate before operations
- Turn off the LED when NONE mode is selected instead of during updates
- Change LED pin type from uint16_t to int to support negative invalid value
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/utility/LEDFeedback.h | Adds invalid pin constant, changes pin type to int, and initializes with invalid value |
| src/utility/LEDFeedback.cpp | Adds pin validation checks and moves LED off logic to mode selection |
| uint32_t* _framePtr = nullptr; | ||
| int32_t _ledChangeInterval = 500; | ||
| int32_t _ledChangeInterval = 0; |
There was a problem hiding this comment.
Changing the default value of _ledChangeInterval from 500 to 0 may have unintended side effects. The original value of 500 appears to be a meaningful default interval, and setting it to 0 could affect existing behavior where this interval is expected to have a non-zero value.
Suggested change
| int32_t _ledChangeInterval = 0; | |
| int32_t _ledChangeInterval = 500; |
Copilot uses AI. Check for mistakes.
|
why not checking for |
Because it's equivalent, but if it's not clear I will update the code. Edit: updated for checking the mode |
|
Memory usage change @ f20103f
Click for full report table
Click for full report CSV |
pennam
approved these changes
Aug 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
It has been reported a malfunction of the OPTA relay number 1 that was continously turning on and off.
The device was running a Cloud Sketch 2.0 with the NetworkConfigurator enabled.
After investigation the problem was found on a not properly initialization of the pin for the feedback led when no feedback mode is selected.
Changes
_ledPinwith an invalid value and check its value before setting itONorOFFNONEis selected